Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tidy imports and context use #266

Closed
wants to merge 2 commits into from
Closed

Tidy imports and context use #266

wants to merge 2 commits into from

Conversation

anacrolix
Copy link
Contributor

No description provided.

@anacrolix anacrolix requested a review from raulk February 15, 2019 04:21
@ghost ghost assigned anacrolix Feb 15, 2019
@ghost ghost added the status/in-progress In progress label Feb 15, 2019

ctx, cancel := context.WithCancel(ctx)
defer cancel()

runner := newQueryRunner(q)
return runner.Run(ctx, peers)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I did a quick sanity check and dhtQueryRunner.Run is broken. It should be creating a cancelable context and canceling it but it's not. See:

go-libp2p-kad-dht/query.go

Lines 122 to 123 in 931ed22

r.runCtx = ctx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Stebalien thanks! I'll look into it more and fix that more directly.

@anacrolix
Copy link
Contributor Author

I need #264 to be merged first, as there's some overlap if the context use is to be cleaned up here.

@anacrolix
Copy link
Contributor Author

I'll hold off on this, I have more important things to work on.

@anacrolix anacrolix closed this Feb 20, 2019
@ghost ghost removed the status/in-progress In progress label Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants